Build system changes to allow compiler toolchain override #454

akosthekiss · 2015-07-22T22:09:58Z

E.g., works with Clang as

make clean && make debug.linux CC=clang-3.5 CXX=clang++-3.5 AR=/usr/bin/ar RANLIB=/usr/bin/ranlib \
  EXTRA_CFLAGS="-Wno-unknown-warning-option -Wno-sign-conversion -Wno-sign-compare -Wno-nested-anon-types -Wno-c99-extensions -Wno-float-conversion -Wno-null-conversion -Wno-invalid-noreturn" \
  LTO=OFF

(Works on x86_64/linux at least.)

akosthekiss · 2015-07-22T22:16:50Z

Also enables the compilation with afl-fuzzer's wrapped gcc/g++ ( #333 ) but link fails because generated code depends on functions available in the compiler-default libc only.

Blocked by #424

ruben-ayrapetyan · 2015-07-23T18:20:08Z

build/configs/toolchain_linux_x86_64.cmake

-find_program(CMAKE_C_COMPILER NAMES x86_64-linux-gnu-gcc x86_64-unknown-linux-gnu-gcc)
-find_program(CMAKE_CXX_COMPILER NAMES x86_64-linux-gnu-g++ x86_64-unknown-linux-gnu-g++)
+if(NOT DEFINED CMAKE_C_COMPILER)
+  find_program(CMAKE_C_COMPILER NAMES x86_64-linux-gnu-gcc x86_64-unknown-linux-gnu-gcc)


@akiss77, maybe, we should introduce some set of toolchain_*.cmake files for different compilers. The files would set compiler executable names and also apply compiler-specific values of build options (for example FLAGS_LTO=-flto for gcc / g++) / warnings configuration etc.
Also, maybe we should extract the architecture-specific part to sub-configuration files for each supported compiler-architecture pair and put compiler-architecture specific flags / options to the files.

In the case, we could remove gcc / g++ specific if blocks from CMakeLists.txt, and also would support clang build for arhitectures other than x86_64.

What do you think about this?

@ruben-ayrapetyan, having "worked-for-us" defaults in the build system is a good thing (actually, that's what we already have in the cmake listfile) but not letting developers deviate from the defaults (i.e., "set compiler executable names") is not, IMHO. There will always be platforms and/or tool chains what we don't have access to (or setups which slightly differ from our development environment, see #489 ). I'd definitely vote for a solution where developers have the maximum freedom to choose tools and settings for the build if they want to adapt to their environment or simply experiment.

@akiss77, I agree to you that we should provide maximum freedom to choose build tools and settings.

I think, we should make the choice as simple, as possible.
To make choice simple, we should provide a simple way of switching to another tool (from predefined set or some new / unknown).

In current version of CMakeLists.txt there are some dependencies on gcc / g++.
I think that the dependencies should be moved to configuration files, external to CMakeLists.txt and also add another necessary configurations (currently, for clang).
In the way, we could save logic and structure of CMakeLists.txt.

In the case, any developer would have simple way to add support of another compiler, without changing logic of CMakeLists.txt.

If there would be some features that would not be provided up to the moment, the CMakeLists.txt could be generalized for the cases.

What do you think about this?

Would the above-described approach allow overriding the defaults from command line as usually expected to work with make, e.g., make CC=xxx CFLAGS=xxx or only by defining configuration files?

galpeter · 2015-07-24T11:11:52Z

FYI: missing sign-off dco.

E.g., works with Clang as ``` make clean && make debug.linux CC=clang-3.5 CXX=clang++-3.5 AR=/usr/bin/ar RANLIB=/usr/bin/ranlib \ EXTRA_CFLAGS="-Wno-unknown-warning-option -Wno-sign-conversion -Wno-sign-compare -Wno-nested-anon-types -Wno-c99-extensions -Wno-float-conversion -Wno-null-conversion -Wno-invalid-noreturn" \ LTO=OFF ``` (Works on x86_64/linux at least.) JerryScript-DCO-1.0-Signed-off-by: Akos Kiss [email protected]

egavrin · 2015-07-29T13:40:43Z

@akiss77 which commonly accepted flags from the list below do you like to support and what behavior do you expect?

AS
CC
CXX
CFLAGS
CXXFLAGS
LDFLAGS
ASFLAGS

akosthekiss · 2015-07-29T15:08:02Z

CC, CFLAGS, CXX, CXXFLAGS, and LDFLAGS would certainly be good, while the definition of AR and RANLIB seems also be needed sometimes (although quite some documentation discourages their definition saying that CMake should figure that out by itself). AS and ASFLAGS are also nice to have (for the sake of completeness), however, I haven't seen a case where they would have been needed yet.

As for the behaviour: use the specified compilers and archive tools, as well as use the specified compiler/linker options instead of the defaults. (Same for the assembler if implemented.)

egavrin · 2015-07-29T16:37:49Z

@akiss77 great! We see this the same way. Please update this PR with support of CC, CFLAGS, CXX, CXXFLAGS, and LDFLAGS - I agree that other flags are not needed yet. Provided options should override defaults.

tilmannOSG · 2015-09-04T10:48:21Z

Can you please elaborate why we need to be able to specify the path to the compiler binary (and the other build tools) via environment variables?

With CMake-based projects you usually just set the path with -DCMAKE_C_COMPILER=... when you configure the project.

egavrin · 2015-10-26T11:09:11Z

@akiss77 do you have any plans to update this PR?

tilmannOSG · 2015-11-20T11:09:40Z

@egavrin I had an offline discussion about this with Akos a while ago. The conclusion was that this can be closed.

Related issue: #454 Works only with default libc: $ make debug.linux -j TOOLCHAIN="./build/configs/toolchain_afl.cmake" USE_COMPILER_DEFAULT_LIBC=YES JerryScript-DCO-1.0-Signed-off-by: Evgeny Gavrin [email protected]

Related issue: #454 Works only with default libc: $ make debug.linux -j TOOLCHAIN="./build/configs/toolchain_afl.cmake" USE_COMPILER_DEFAULT_LIBC=YES JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan [email protected] JerryScript-DCO-1.0-Signed-off-by: Evgeny Gavrin [email protected]

Related issue: #333, #454 Works only with default libc: $ make debug.linux -j TOOLCHAIN="./build/configs/toolchain_afl.cmake" USE_COMPILER_DEFAULT_LIBC=YES JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan [email protected] JerryScript-DCO-1.0-Signed-off-by: Evgeny Gavrin [email protected]

Related issue: #333, #454 Works only with default libc: ``` $ make debug.linux -j TOOLCHAIN="./build/configs/toolchain_afl.cmake" USE_COMPILER_DEFAULT_LIBC=YES ``` JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan [email protected] JerryScript-DCO-1.0-Signed-off-by: Evgeny Gavrin [email protected]

egavrin · 2015-11-23T12:17:40Z

Closed after offline discussion.

Related issue: #333, #454 Works only with default libc: ``` $ make debug.linux -j TOOLCHAIN="./build/configs/toolchain_afl.cmake" USE_COMPILER_DEFAULT_LIBC=YES ``` JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan [email protected] JerryScript-DCO-1.0-Signed-off-by: Evgeny Gavrin [email protected]

Related issue: jerryscript-project#333, jerryscript-project#454 Works only with default libc: ``` $ make debug.linux -j TOOLCHAIN="./build/configs/toolchain_afl.cmake" USE_COMPILER_DEFAULT_LIBC=YES ``` JerryScript-DCO-1.0-Signed-off-by: Ruben Ayrapetyan [email protected] JerryScript-DCO-1.0-Signed-off-by: Evgeny Gavrin [email protected]

ruben-ayrapetyan reviewed Jul 23, 2015
View reviewed changes

galpeter added enhancement An improvement infrastructure Related to GH Actions or the tested targets labels Jul 24, 2015

akosthekiss force-pushed the compiler-override branch from 454ca38 to 9b12516 Compare July 29, 2015 08:52

egavrin added this to the Engine optimization & enhancement milestone Jul 29, 2015

egavrin self-assigned this Jul 29, 2015

egavrin assigned akosthekiss and unassigned egavrin Jul 30, 2015

egavrin added the critical Raises security concerns label Aug 3, 2015

egavrin mentioned this pull request Nov 23, 2015

Add support of afl-fuzzer #731

Merged

egavrin closed this Nov 23, 2015

akosthekiss deleted the compiler-override branch November 16, 2017 17:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Build system changes to allow compiler toolchain override #454

Build system changes to allow compiler toolchain override #454

Uh oh!

akosthekiss commented Jul 22, 2015

Uh oh!

akosthekiss commented Jul 22, 2015

Uh oh!

ruben-ayrapetyan Jul 23, 2015

Uh oh!

akosthekiss Jul 29, 2015

Uh oh!

ruben-ayrapetyan Jul 29, 2015

Uh oh!

akosthekiss Jul 29, 2015

Uh oh!

galpeter commented Jul 24, 2015

Uh oh!

egavrin commented Jul 29, 2015

Uh oh!

akosthekiss commented Jul 29, 2015

Uh oh!

egavrin commented Jul 29, 2015

Uh oh!

tilmannOSG commented Sep 4, 2015

Uh oh!

egavrin commented Oct 26, 2015

Uh oh!

tilmannOSG commented Nov 20, 2015

Uh oh!

egavrin commented Nov 23, 2015

Uh oh!

Uh oh!

Build system changes to allow compiler toolchain override #454

Build system changes to allow compiler toolchain override #454

Uh oh!

Conversation

akosthekiss commented Jul 22, 2015

Uh oh!

akosthekiss commented Jul 22, 2015

Uh oh!

ruben-ayrapetyan Jul 23, 2015

Choose a reason for hiding this comment

Uh oh!

akosthekiss Jul 29, 2015

Choose a reason for hiding this comment

Uh oh!

ruben-ayrapetyan Jul 29, 2015

Choose a reason for hiding this comment

Uh oh!

akosthekiss Jul 29, 2015

Choose a reason for hiding this comment

Uh oh!

galpeter commented Jul 24, 2015

Uh oh!

egavrin commented Jul 29, 2015

Uh oh!

akosthekiss commented Jul 29, 2015

Uh oh!

egavrin commented Jul 29, 2015

Uh oh!

tilmannOSG commented Sep 4, 2015

Uh oh!

egavrin commented Oct 26, 2015

Uh oh!

tilmannOSG commented Nov 20, 2015

Uh oh!

egavrin commented Nov 23, 2015

Uh oh!

Uh oh!